fix: remove anyhow feature flag from OptionExt location test
#148
Conversation
|
I don't think this requires a changelog entry. |
|
I see, good catch. I recently learnt that conditional trait methods are a backwards compatability nightmare (though WrapErr is sealed thankfully), as there was a recent wasmtime discussion regarding failure to compile because a trait implemented new methods another crate didn't implement under a certain feature flag combiniation. A better option would be to move these to a supertrait or ContextCompat, and make trait implement for all errors to expose the same behavior as present, without footguns. |
|
That would be an entirely different changeset, though, right? |
|
Yes, just a proposition, as it would alleviate the confusion with two identically named methods from two different traits. From what I see in the current source code If you are willing to do this, or want me to do this, just let me know. It is a larger change, but would alleviate some of the confusion and import confusion for downstream users |
|
Got my idea here: #150 I feel I need to clarify that this discussion is more about 8522f02 than the removal of the feature gate of the Just doing what I can to clear out any confusion and make eyre as consistent as possible while we can. In short, we move the When the anyhow feature drops from default features, errors and anyhow+options will be more self-contained and not lead to the same Let me know what you think of this. |
LeoniePhiline
added
C-bug
Both `WrapErr` and `ContextCompat` expose `anyhow` compatibility methods. `ContextCompat` is completely feature gated behind the `anyhow` flag. `WrapErr`, on the other hand, feature-gates individual functions behind the `anyhow` flag. This has led to [confusion][confusion] in the past. [confusion]: eyre-rs#138 (comment)
thenorili
force-pushed
the
fix/test-Option-ok_or_eyre-remove-cfg-anyhow
branch
from
68744f1
to
fd4ea39
Compare
|
Thank you for your work Leonie. Sorry for leaving this hanging, had a rough time lately, but I am back now 🎉 |
|
Please do not worry at all! Your wellbeing is always more important. <3 |
1. Clarify trait usage in location test
8522f02Both
WrapErrandContextCompatexposeanyhowcompatibility methods.ContextCompatis completely feature gated behind theanyhowflag.WrapErr, on the other hand, feature-gates individual functions behind theanyhowflag.This has led to confusion in the past.
This change moves the
use eyre::WrapErrstatement from the top of the module into each individual test, as identically named functions (wrap_err.*) are exposed both byWrapErrandContextCompat.With
use eyre::WrapErrmoved directly into the tests, it becomes clear which trait-providedfnis being called.2. Remove
anyhowfeature flag fromOptionExtlocation test68744f1This bug was introduced (by me) in 34bd1d9#diff-1ff47dac6cf55e34ff587968c5b1f1ec6b6ae39d2668a66ecba3633163a21fc5R86 - partly due to the confusion fixed with the above commit.
Fixes #147